Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Feature/internal/sync map #2109

Closed
wants to merge 10 commits into from
Closed

Conversation

ykadowak
Copy link
Contributor

@ykadowak ykadowak commented Jul 5, 2023

Description:

Related Issue:

Versions:

  • Go Version: 1.20.5
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.27.3
  • NGT Version: 2.0.13

Checklist:

Special notes for your reviewer:

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 5, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9dee4b0
Status: ✅  Deploy successful!
Preview URL: https://54f8c741.vald.pages.dev
Branch Preview URL: https://feature-internal-sync-map.vald.pages.dev

View logs

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 5, 2023

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

type cache struct {
gache gache.Gache
type cache[V any] struct {
gache gache.Gache[V]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
gache is unused (structcheck)

@@ -36,7 +38,7 @@ type Group[V any] interface {
}

type group[V any] struct {
m sync.Map
m valdsync.Map[string, *call[V]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
m is unused (structcheck)

@@ -0,0 +1,7 @@
package sync

import gache "github.com/kpango/gache/v2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
import 'github.com/kpango/gache/v2' is not allowed from list 'Main' (depguard)

c.gache.StartExpired(ctx, c.expireCheckDur)
}

// Get calls StartExpired func of c.gache and returns (interface{}, bool) according to key.
func (c *cache) Get(key string) (interface{}, bool) {
func (c *cache[V]) Get(key string) (V, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Get returns interface (V) (ireturn)

c.gache.Delete(key)
}

// GetAndDelete returns (interface{}, bool) and delete value according to key when value of key is set.
// When value of key is not set, returns (nil, false).
func (c *cache) GetAndDelete(key string) (interface{}, bool) {
func (c *cache[V]) GetAndDelete(key string) (V, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
GetAndDelete returns interface (V) (ireturn)

@@ -554,7 +554,7 @@ func Test_cache_GetAndDelete(t *testing.T) {
test := tc
t.Run(test.name, func(tt *testing.T) {
defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
c := &cache{
c := &cache[any]{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
variable name 'c' is too short for the scope of its usage (varnamelen)

@@ -119,7 +120,7 @@ func TestWithGache(t *testing.T) {

tests := []test{
func() test {
ga := gache.New()
ga := gache.New[any]()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
variable name 'ga' is too short for the scope of its usage (varnamelen)

beforeFunc func(args)
afterFunc func(args)
}
defaultCheckFunc := func(w want, c *cache) error {
defaultCheckFunc := func(w want, c *cache[any]) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
parameter name 'w' is too short for the scope of its usage (varnamelen)

beforeFunc func(args)
afterFunc func(args)
}

defaultCheckFunc := func(w want, got *cache) error {
defaultCheckFunc := func(w want, got *cache[any]) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
parameter name 'w' is too short for the scope of its usage (varnamelen)

defaultCheckFunc := func(w want, got cacher.Cache) error {
wc := reflect.ValueOf(w.wantC.(*cache))
gc := reflect.ValueOf(got.(*cache))
defaultCheckFunc := func(w want, got cacher.Cache[any]) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
parameter name 'w' is too short for the scope of its usage (varnamelen)

import gache "github.com/kpango/gache/v2"

type Map[K comparable, V any] struct {
gache.Map[K,V]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
File is not gofumpt-ed (gofumpt)

@@ -34,6 +34,7 @@ import (
"github.com/vdaas/vald/internal/observability/trace"
"github.com/vdaas/vald/internal/safety"
"github.com/vdaas/vald/internal/tls"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
File is not goimports-ed (goimports)

@@ -454,7 +454,7 @@ func Test_cache_Delete(t *testing.T) {
test := tc
t.Run(test.name, func(tt *testing.T) {
defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
c := &cache{
c := &cache[any]{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
variable name 'c' is too short for the scope of its usage (varnamelen)

@@ -360,7 +360,7 @@
if test.checkFunc == nil {
checkFunc = defaultCheckFunc
}
c := &cache{
c := &cache[any]{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
variable name 'c' is too short for the scope of its usage (varnamelen)

afterFunc func(args)
}
defaultCheckFunc := func(w want, c *cache) error {
defaultCheckFunc := func(w want, c *cache[any]) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
parameter name 'w' is too short for the scope of its usage (varnamelen)

@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch coverage: 86.20% and project coverage change: +0.09 🎉

Comparison is base (a46ac9e) 29.89% compared to head (9dee4b0) 29.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2109      +/-   ##
==========================================
+ Coverage   29.89%   29.98%   +0.09%     
==========================================
  Files         369      369              
  Lines       35091    35083       -8     
==========================================
+ Hits        10489    10519      +30     
+ Misses      24118    24083      -35     
+ Partials      484      481       -3     
Impacted Files Coverage Δ
internal/cache/cacher/cacher.go 100.00% <ø> (ø)
internal/circuitbreaker/manager.go 0.00% <0.00%> (ø)
internal/client/v1/client/discoverer/discover.go 0.00% <0.00%> (ø)
internal/client/v1/client/filter/egress/client.go 0.00% <ø> (ø)
internal/client/v1/client/filter/ingress/client.go 0.00% <ø> (ø)
internal/net/grpc/client.go 0.00% <0.00%> (ø)
pkg/gateway/lb/handler/grpc/aggregation.go 0.00% <ø> (ø)
pkg/gateway/lb/service/gateway.go 0.00% <0.00%> (ø)
pkg/manager/index/service/indexer.go 0.00% <ø> (ø)
internal/cache/cache.go 100.00% <100.00%> (ø)
... and 5 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -266,7 +266,7 @@ func Test_cache_Get(t *testing.T) {
test := tc
t.Run(test.name, func(tt *testing.T) {
defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
c := &cache{
c := &cache[any]{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
variable name 'c' is too short for the scope of its usage (varnamelen)

@ykadowak
Copy link
Contributor Author

continue with #2115

@ykadowak ykadowak closed this Jul 10, 2023
@kpango kpango deleted the feature/internal/sync-map branch May 21, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants